hyperscan: add caching mechanism for hyperscan contexts v5 #11811

lukashino · 2024-09-23T11:17:59Z

Followup of #11774

Cache Hyperscan serialized databases to disk to prevent compilation of the same databases when Suricata is run again with the same ruleset.
The current work operates in the logging folder and caches individual Hyperscan databases - potentially the ruleset might be even slightly changed and it still can reuse part of the unchanged signature groups.
Loading fresh ET Open ruleset: 19 seconds
Loading cached ET Open ruleset: 07 seconds

Link to ticket: https://redmine.openinfosecfoundation.org/issues/7170

Describe changes:
v5:

rebased
commit message update
docs update

v4:

rebased
changed the default caching directory to somewhere /var/lib/suricata/cache/hs
custom cache directory path option added
docs added
the default settings changed - enabled on the config generation, disabled when the option is not present in the config

v3

rebased
MPM caching is still left on by default.

v2

improved styling to follow Suricata code styleguide
increased cache file name length from 10 to 20 characters
cache file name is a hash of the patterns - now only HS relevant fields are hashed - as long as the group of patterns itself is not changed then it is reused
minor refactors
added a safe variant of littlehash2 function
added suricata.yaml option to enable/disable caching
changed the storage location to the configured logging directory

v1

initial work to cache and load Hyperscan databases from the disk

This variant of hashlittle2() ensures that it avoids accesses beyond the last byte of the string, which will cause warnings from tools like Valgrind or Address Sanitizer.

Cache Hyperscan serialized databases to disk to prevent compilation of the same databases when Suricata is run again with the same ruleset. The current work operates in /tmp/ folder and caches individual signature group heads - potentially the ruleset might be even slightly changed and it still can reuse part of the unchanged signature groups. Loading *fresh* ET Open ruleset: 19 seconds Loading *cached* ET Open ruleset: 07 seconds

codecov · 2024-09-23T11:36:32Z

Codecov Report

Attention: Patch coverage is 1.78042% with 331 lines in your changes missing coverage. Please review.

Project coverage is 82.49%. Comparing base (7ab8334) to head (02921a5).
Report is 28 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #11811      +/-   ##
==========================================
- Coverage   82.58%   82.49%   -0.10%     
==========================================
  Files         914      914              
  Lines      249500   249821     +321     
==========================================
+ Hits       206045   206078      +33     
- Misses      43455    43743     +288

Flag	Coverage Δ
fuzzcorpus	`60.51% <0.00%> (+0.08%)`	⬆️
livemode	`18.85% <0.00%> (+0.13%)`	⬆️
pcap	`43.89% <0.00%> (-0.19%)`	⬇️
suricata-verify	`61.98% <0.00%> (-0.05%)`	⬇️
unittests	`58.89% <1.78%> (-0.08%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

jufajardini · 2024-09-25T14:49:10Z

suricata.yaml.in

@@ -1685,6 +1685,10 @@ detect:
    toclient-groups: 3
    toserver-groups: 25
  sgh-mpm-context: auto
+  # Cache MPM contexts to the disk for avoid rule compilation at the startup.


interestingly, this version had better preposition usage :P

jufajardini

Commit explanations look good to me, considering past reviews.

I did notice however that we failed to notice that they're not referencing the redmine ticket >_<'

jufajardini · 2024-09-25T14:52:33Z

doc/userguide/performance/hyperscan.rst

+
+**Note**: 
+You might need create and adjust permissions to the default caching folder path, 


nit:

Suggested change

You might need create and adjust permissions to the default caching folder path,

You might need to create and adjust permissions to the default caching folder path,

jasonish · 2024-09-25T19:07:27Z

Hitting an issue when running with privilege dropping. For example:

remove the cache directory: sudo rm -rf var/lib/suricata/cache/
recreate as if setup for privilege dropping:
- sudo mkdir var/lib/suricata/cache
- sudo chown -R root:jason var/lib/suricata/cache
- sudo chmod 2775 var/lib/suricata/cache
Run Suricata: sudo ./src/suricata -i enp10s0 --user jason

Results in (would be many more but we limit to one warning):

Warning: mpm-hs: Failed to create Hyperscan cache file, make sure the folder exist and is writable or adjust sgh-mpm-caching-path setting (/opt/suricata/8.0.0-dev/var/lib/suricata/cache/hs/17049829294979608135_v1.hs) [HSSaveCache:util-mpm-hs.c:765]

It appears the caching is done before privileges are dropped, and there is no access after privileges are dropped.

And then during a rule reload, where privileges are dropped, we don't have access to this directory.

jasonish · 2024-09-25T19:31:01Z

Hitting an issue when running with privilege dropping. For example:
* remove the cache directory: `sudo rm -rf var/lib/suricata/cache/`

* recreate as if setup for privilege dropping:
  
  * `sudo mkdir var/lib/suricata/cache`
  * `sudo chown -R root:jason var/lib/suricata/cache`
  * `sudo chmod 2775 var/lib/suricata/cache`

* Run Suricata: `sudo ./src/suricata -i enp10s0 --user jason`

* Results in (would be many more but we limit to one warning):
  ```
  Warning: mpm-hs: Failed to create Hyperscan cache file, make sure the folder exist and is writable or adjust sgh-mpm-caching-path setting (/opt/suricata/8.0.0-dev/var/lib/suricata/cache/hs/17049829294979608135_v1.hs) [HSSaveCache:util-mpm-hs.c:765]
  ```
It appears the caching is done before privileges are dropped, and there is no access after privileges are dropped.

And then during a rule reload, where privileges are dropped, we don't have access to this directory.

Ok, so the directory creation is done before privilege dropping. If that can be delayed til after privilege dropping we should be good. For example, the file store directory layout is made after dropping privileges.

jasonish

Comments in-line with respect to some required changes when running with privilege dropping.

suricata-qa · 2024-09-26T23:40:03Z

WARNING:

field	baseline	test	%
	SURI_TLPR1_stats_chk
.uptime	649	622	95.84%

Pipeline 22886

lukashino · 2024-10-06T21:53:02Z

goto #11887

Lukas Sismis added 4 commits September 23, 2024 13:07

hashlittle: add a safe variant of hashlittle2 function

28c5373

This variant of hashlittle2() ensures that it avoids accesses beyond the last byte of the string, which will cause warnings from tools like Valgrind or Address Sanitizer.

util-path: remove dead code

653466c

detect-engine: remove commented out code

02921a5

lukashino requested review from jufajardini, victorjulien and a team as code owners September 23, 2024 11:18

jufajardini reviewed Sep 25, 2024

View reviewed changes

jasonish requested changes Sep 25, 2024

View reviewed changes

lukashino mentioned this pull request Oct 6, 2024

hyperscan: add caching mechanism for hyperscan contexts v7 #11887

Closed

lukashino closed this Oct 6, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

hyperscan: add caching mechanism for hyperscan contexts v5 #11811

hyperscan: add caching mechanism for hyperscan contexts v5 #11811

lukashino commented Sep 23, 2024

codecov bot commented Sep 23, 2024 •

edited

Loading

jufajardini Sep 25, 2024

jufajardini left a comment

jufajardini Sep 25, 2024

jasonish commented Sep 25, 2024

jasonish commented Sep 25, 2024

jasonish left a comment

suricata-qa commented Sep 26, 2024

lukashino commented Oct 6, 2024


		Note:
		You might need create and adjust permissions to the default caching folder path,

	You might need create and adjust permissions to the default caching folder path,
	You might need to create and adjust permissions to the default caching folder path,

hyperscan: add caching mechanism for hyperscan contexts v5 #11811

hyperscan: add caching mechanism for hyperscan contexts v5 #11811

Conversation

lukashino commented Sep 23, 2024

codecov bot commented Sep 23, 2024 • edited Loading

Codecov Report

jufajardini Sep 25, 2024

Choose a reason for hiding this comment

jufajardini left a comment

Choose a reason for hiding this comment

jufajardini Sep 25, 2024

Choose a reason for hiding this comment

jasonish commented Sep 25, 2024

jasonish commented Sep 25, 2024

jasonish left a comment

Choose a reason for hiding this comment

suricata-qa commented Sep 26, 2024

lukashino commented Oct 6, 2024

codecov bot commented Sep 23, 2024 •

edited

Loading